Skip to content

Conversation

@hersle
Copy link
Contributor

@hersle hersle commented Sep 30, 2025

I want to fix #3871.

I figured torn_system_jacobian_sparsity computes an incorrect sparsity pattern for this example.
My first idea is to disable it and always compute jacobian_sparsity with the "fallback method" Symbolics.jacobian_sparsity(...).
If this does not work in all cases, the alternative would be to patch torn_system_jacobian_sparsity directly.

@hersle hersle force-pushed the jac_observed_derivative branch from 4d13c7a to 0dac698 Compare September 30, 2025 09:44
@hersle hersle force-pushed the jac_observed_derivative branch from 0dac698 to 16c9c0d Compare September 30, 2025 10:42
@hersle
Copy link
Contributor Author

hersle commented Sep 30, 2025

It looks to me like the relevant tests are passing. Do you have any input on this @ChrisRackauckas?

Comment on lines -458 to +460
sparsity = torn_system_jacobian_sparsity(sys)
sparsity === nothing || return sparsity
# disable to fix https://github.com/SciML/ModelingToolkit.jl/issues/3871
#sparsity = torn_system_jacobian_sparsity(sys)
#sparsity === nothing || return sparsity
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AayushSabharwal is there a reason the torn sparsity would have false zeros here? That seems like it would give issues in other places.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure, we've just used torn_system_jacobian_sparsity since the beginning of time iirc. Maybe this doesn't handle the W sparsity correctly?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I would think that if this sparsity pattern is wrong, then the incidence matrix is wrong. Can you make a note to follow up on this? For now I want to merge so that these are right, but I'd like to make sure something isn't hiding here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

@ChrisRackauckas ChrisRackauckas merged commit 2d1f2ff into SciML:master Oct 1, 2025
37 of 51 checks passed
@hersle
Copy link
Contributor Author

hersle commented Oct 1, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sparse ODE segfaults when RHS contains differentiated variable

3 participants